-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(samgongustofa): Include operators and co-owners in validation + other minor fixes #17609
fix(samgongustofa): Include operators and co-owners in validation + other minor fixes #17609
Conversation
…esh (add findVehicle field into pickVehicle)
WalkthroughThe pull request introduces widespread modifications across multiple application templates and services, focusing on enhancing vehicle and machine detail retrieval. The primary change involves replacing direct property access with the Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 10 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
…ners-in-validation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (7)
libs/application/templates/energy-funds/src/utils/getSelectedVehicle.ts (1)
12-13
: Consider handling potential undefined values forcurrentVehicleList
.
If the external data is missing or malformed,currentVehicleList
may beundefined
and calling.vehicles.find(...)
could cause a runtime error. Consider using optional chaining or verifyingcurrentVehicleList
is defined before accessing itsvehicles
array.libs/application/templates/transport-authority/order-vehicle-registration-certificate/src/utils/getSelectedVehicle.ts (1)
9-11
: Return handling for missing vehicle data.
Returning{}
when no matching vehicle is found could lead to unexpected behavior if the calling code depends on specific vehicle properties. Consider returningundefined
or a more explicit indicator when a vehicle is not found.libs/application/templates/transport-authority/change-co-owner-of-vehicle/src/utils/getSelectedVehicle.ts (1)
9-11
: Avoid mutating thecurrentVehicleList
array directly.
The inline update tocurrentVehicleList.vehicles[index]
can be replaced by creating a new updated array to maintain immutability, prevent unexpected side effects, and improve maintainability.libs/application/templates/transport-authority/transfer-of-vehicle-ownership/src/utils/getSelectedVehicle.ts (1)
9-11
: Use explicit boolean checks for clarity.The new usage of
getValueViaPath<boolean | undefined>(answers, 'pickVehicle.findVehicle')
improves path-based retrieval. For added clarity, consider explicitly checking the boolean value (e.g.,=== true
) to avoid inadvertently treatingundefined
as a falsy value.libs/application/templates/transport-authority/change-operator-of-vehicle/src/utils/getSelectedVehicle.ts (1)
9-11
: Maintain consistent boolean evaluation.Similar to other files, ensure that the boolean check against
getValueViaPath
is explicit if you want to guard againstundefined
. This helps future maintainers avoid confusion about falsy comparisons.libs/clients/transport-authority/vehicle-owner-change/src/lib/vehicleOwnerChangeClient.service.ts (1)
117-127
: Replace null with empty arrays for operators/coOwners
Passingnull
might be acceptable if the API mandates it, but returning empty arrays often simplifies downstream handling. Confirm API expectations or consider returning empty arrays to avoid potential null checks.operatorEmail: ownerChange.operators?.find((x) => x.isMainOperator)?.email || null, operators: - ownerChange.operators?.map((operator) => {...}) || null, + ownerChange.operators?.map((operator) => {...}) || [], coOwners: - ownerChange.coOwners?.map((coOwner) => {...}) || null, + ownerChange.coOwners?.map((coOwner) => {...}) || [],libs/application/ui-fields/src/lib/FindVehicleFormField/FindVehicleFormField.tsx (1)
297-297
: Consider refactoring repeated logic
You repeat the samesetValue(\
${field.id}.findVehicle`, true)operation in each
set*Values` function. Factor out a helper if this pattern grows or changes frequently to enhance maintainability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
libs/application/template-api-modules/src/lib/modules/templates/energy-funds/energy-funds.service.ts
(1 hunks)libs/application/templates/aosh/change-machine-supervisor/src/utils/getSelectedMachine.ts
(1 hunks)libs/application/templates/aosh/deregister-machine/src/utils/getSelectedMachine.ts
(1 hunks)libs/application/templates/aosh/request-for-inspection/src/utils/getSelectedMachine.ts
(1 hunks)libs/application/templates/aosh/street-registration/src/utils/getSelectedMachine.ts
(1 hunks)libs/application/templates/aosh/transfer-of-machine-ownership/src/utils/getSelectedMachine.ts
(1 hunks)libs/application/templates/energy-funds/src/utils/getSelectedVehicle.ts
(1 hunks)libs/application/templates/transport-authority/change-co-owner-of-vehicle/src/forms/ChangeCoOwnerOfVehicleForm/InformationSection/vehicleSubSection.ts
(1 hunks)libs/application/templates/transport-authority/change-co-owner-of-vehicle/src/utils/getSelectedVehicle.ts
(1 hunks)libs/application/templates/transport-authority/change-operator-of-vehicle/src/forms/ChangeOperatorOfVehicleForm/InformationSection/vehicleSubSection.ts
(1 hunks)libs/application/templates/transport-authority/change-operator-of-vehicle/src/utils/getSelectedVehicle.ts
(1 hunks)libs/application/templates/transport-authority/order-vehicle-license-plate/src/utils/getSelectedVehicle.ts
(1 hunks)libs/application/templates/transport-authority/order-vehicle-registration-certificate/src/utils/getSelectedVehicle.ts
(1 hunks)libs/application/templates/transport-authority/transfer-of-vehicle-ownership/src/fields/CoOwner/index.tsx
(3 hunks)libs/application/templates/transport-authority/transfer-of-vehicle-ownership/src/forms/TransferOfVehicleOwnershipForm/InformationSection/vehicleSubSection.ts
(1 hunks)libs/application/templates/transport-authority/transfer-of-vehicle-ownership/src/utils/getSelectedVehicle.ts
(1 hunks)libs/application/ui-fields/src/lib/FindVehicleFormField/FindVehicleFormField.tsx
(3 hunks)libs/clients/transport-authority/vehicle-owner-change/src/lib/vehicleOwnerChangeClient.service.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (18)
libs/application/templates/aosh/change-machine-supervisor/src/utils/getSelectedMachine.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/order-vehicle-registration-certificate/src/utils/getSelectedVehicle.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/aosh/request-for-inspection/src/utils/getSelectedMachine.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/aosh/street-registration/src/utils/getSelectedMachine.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/change-co-owner-of-vehicle/src/utils/getSelectedVehicle.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/order-vehicle-license-plate/src/utils/getSelectedVehicle.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/change-co-owner-of-vehicle/src/forms/ChangeCoOwnerOfVehicleForm/InformationSection/vehicleSubSection.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/aosh/deregister-machine/src/utils/getSelectedMachine.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/transfer-of-vehicle-ownership/src/utils/getSelectedVehicle.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/aosh/transfer-of-machine-ownership/src/utils/getSelectedMachine.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/change-operator-of-vehicle/src/forms/ChangeOperatorOfVehicleForm/InformationSection/vehicleSubSection.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/template-api-modules/src/lib/modules/templates/energy-funds/energy-funds.service.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/ui-fields/src/lib/FindVehicleFormField/FindVehicleFormField.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/clients/transport-authority/vehicle-owner-change/src/lib/vehicleOwnerChangeClient.service.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/transfer-of-vehicle-ownership/src/forms/TransferOfVehicleOwnershipForm/InformationSection/vehicleSubSection.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/transfer-of-vehicle-ownership/src/fields/CoOwner/index.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/change-operator-of-vehicle/src/utils/getSelectedVehicle.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/energy-funds/src/utils/getSelectedVehicle.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🪛 Biome (1.9.4)
libs/application/templates/transport-authority/transfer-of-vehicle-ownership/src/fields/CoOwner/index.tsx
[error] 61-61: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (14)
libs/application/templates/aosh/change-machine-supervisor/src/utils/getSelectedMachine.ts (1)
10-12
: Adopt consistent fallback handling for optional booleans.The usage of
getValueViaPath<boolean | undefined>
is correct and aligned with the library’s typed approach for nested data retrieval. Consider confirming thatmachine.findVehicle
is indeed a boolean in all cases, ensuring the if-statement logic is unambiguous. Otherwise, this looks good.libs/application/templates/aosh/deregister-machine/src/utils/getSelectedMachine.ts (1)
10-12
: Validate the boolean or undefined return type.Relying on
getValueViaPath<boolean | undefined>
for checkingmachine.findVehicle
is a solid approach that enhances type safety. The fallback behavior is clear if the value is undefined, but consider adding a comment or test to confirm this flow works as intended.libs/application/templates/aosh/request-for-inspection/src/utils/getSelectedMachine.ts (1)
10-12
: Ensure consistent usage throughout.This update aligns with the broader effort to use
getValueViaPath
for nested properties. Keeping the data retrieval approach consistent across modules simplifies maintainability. The change is straightforward and appears correct.libs/application/templates/aosh/transfer-of-machine-ownership/src/utils/getSelectedMachine.ts (1)
10-12
: Check for undefined states.Using
getValueViaPath
avoids direct property access and elegantly handles potential null or undefined states inanswers
. This appears consistent with other modules. Overall, the logic is sound and improves reliability.libs/application/templates/transport-authority/order-vehicle-license-plate/src/utils/getSelectedVehicle.ts (1)
9-11
: Good use ofgetValueViaPath
for nested data retrieval.
This condition appropriately checks for thefindVehicle
flag withinpickVehicle
. The subsequent optional chaining oncurrentVehicleList?.vehicles
also helps prevent runtime errors if external data is missing.libs/application/templates/aosh/street-registration/src/utils/getSelectedMachine.ts (1)
16-16
: Retain type safety when retrieving values.Using
getValueViaPath<boolean | undefined>
increases type safety. Confirm thatmachine.findVehicle
is indeed stored as a boolean to prevent unintentional type coercion and ensure reusability across NextJS apps.libs/application/templates/transport-authority/change-operator-of-vehicle/src/forms/ChangeOperatorOfVehicleForm/InformationSection/vehicleSubSection.ts (1)
72-72
: Good use of required validation for mileage.Adding
required: true
correctly enforces mileage submission. This aligns with the indicated need for mandatory mileage input, improving both user feedback and data integrity.libs/application/templates/transport-authority/change-co-owner-of-vehicle/src/forms/ChangeCoOwnerOfVehicleForm/InformationSection/vehicleSubSection.ts (1)
73-73
: Enforce required mileage field
Addingrequired: true
ensures mileage data is collected whenever needed. This aligns with the PR objective of uniformly enforcing mandatory mileage.libs/application/templates/transport-authority/transfer-of-vehicle-ownership/src/fields/CoOwner/index.tsx (2)
3-3
: Efficient path-based data retrieval
Good use ofgetErrorViaPath
andgetValueViaPath
to dynamically retrieve nested error and value data.
20-20
: Destructure errors for better validation
Destructuringerrors
allows passing them directly to theInputController
. This keeps the component neat.libs/application/template-api-modules/src/lib/modules/templates/energy-funds/energy-funds.service.ts (1)
167-170
: Verify correct type usage for getValueViaPath
getValueViaPath<boolean | undefined>
impliesselectVehicle.findVehicle
might be boolean. Confirm this matches actual data shape to avoid type mismatches.✅ Verification successful
Type usage for getValueViaPath is correct ✅
The typeboolean | undefined
forselectVehicle.findVehicle
is used consistently across the codebase and follows a logical pattern where it acts as a flag indicating vehicle selection.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search usage of selectVehicle.findVehicle to confirm it's boolean or object. rg -A 5 "selectVehicle\.findVehicle"Length of output: 1433
libs/application/ui-fields/src/lib/FindVehicleFormField/FindVehicleFormField.tsx (2)
243-243
: Confirm 'findVehicle' assignment validity
Setting the'findVehicle'
field totrue
unconditionally seems valid for the current context, but ensure that there is no scenario where afalse
value is needed to reset or revert the vehicle lookup status.
323-323
: Maintain consistent approach to 'findVehicle'
This line is consistent with the logic in othersetValues
functions. Once verified that in all scenarios'findVehicle'
should be set totrue
, this approach looks correct.libs/application/templates/transport-authority/transfer-of-vehicle-ownership/src/forms/TransferOfVehicleOwnershipForm/InformationSection/vehicleSubSection.ts (1)
93-93
: Validate compulsory mileage field
Marking the mileage field as required ensures that the user must provide a value. Confirm that an appropriate validation message is displayed and tested so users understand why they need to enter the mileage.
...ion/templates/transport-authority/transfer-of-vehicle-ownership/src/fields/CoOwner/index.tsx
Show resolved
Hide resolved
Datadog ReportAll test runs ✅ 2 Total Test Services: 0 Failed, 2 Passed Test Services
|
…ners-in-validation
…ners-in-validation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…ners-in-validation
…ners-in-validation
…ners-in-validation
…ther minor fixes (#17609) * Include operators and coOwners when calling personCheck (validation) * Add required star to mileage field * Fix getSelectedVehicle since the value findVehicle is cleared on refresh (add findVehicle field into pickVehicle) * Make inputs coOwner email and phone red if failing validation * More fixes for findVehicle * Cleanup
…ther minor fixes (#17609) * Include operators and coOwners when calling personCheck (validation) * Add required star to mileage field * Fix getSelectedVehicle since the value findVehicle is cleared on refresh (add findVehicle field into pickVehicle) * Make inputs coOwner email and phone red if failing validation * More fixes for findVehicle * Cleanup
What
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style